Fix silent failures in publish-wheels.sh (masked twine errors, skipped confirmations)#352
Merged
Conversation
Member
Author
|
Added |
Member
Author
|
Codex adversarial review (single-arm, right-sized for the diff) found the warn-and-continue paths could recreate the partial-publish incident: missing/empty package dirs and declined prompts returned success, and
|
…e (review findings)
…/PECOS into publish-script-robustness
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two robustness bugs in
scripts/publish-wheels.sh, both of which bit during the 0.9.0.dev0 publish:Error-masked twine check. The check piped twine's output through
grep -v license-file, so theiftested grep's exit status — a failed or even missing twine ("Failed to spawn: twine") still printed "Distribution checks passed". Now the output is captured once and twine's own exit status decides (twine exits nonzero on real errors, zero on the benign maturin license-file warning, which is still filtered from the display).Piped confirmation skipped a package. The per-package prompt used
read -n 1, so pipingyesfedyto the first prompt and the leftover\nto the second — during the 0.9.0.dev0 publish this silently skippedpecos-rslib-llvm, briefly leavingquantum-pecoson PyPI with an unresolvable exact pin (caught and uploaded directly). Fixed twofold: the prompt now reads a full line, and a-y/--yesflag supports non-interactive use without piping tricks.Verification:
bash -nclean; side-by-side read semantics demonstrate the old bug and the fix (old read 2: []vsnew read 2: [y]under pipedyes);--dry-runagainst the real 0.9.0.dev0 distribution bundle passes all three package checks with the new logic.